fix(ai-openrouter): preserve disabled reasoning requests - #1013
Merged
AlemTuzlak merged 5 commits intoAug 21, 2026
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 56 minutes Limit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe OpenRouter adapter accepts ChangesOpenRouter reasoning wire format
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant ReasoningWireRoute
participant OpenRouterAdapter
participant AimockJournal
Playwright->>ReasoningWireRoute: POST scenario
ReasoningWireRoute->>OpenRouterAdapter: Stream chat request
OpenRouterAdapter->>AimockJournal: Send normalized or omitted reasoning
AimockJournal-->>ReasoningWireRoute: Return mock stream
ReasoningWireRoute-->>Playwright: Return success
Playwright->>AimockJournal: Inspect captured request
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kinKingen
marked this pull request as ready for review
July 29, 2026 12:37
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-openrouter/tests/openrouter-adapter.test.ts`:
- Around line 1649-1671: Extend the mock-backed E2E route/spec in testing/e2e/
to send a request with reasoning: {} and verify the outbound wire request omits
the reasoning field. Reuse the existing E2E request path and assertions for
reasoning serialization, adding coverage for both the route-level payload and
serialized request if applicable.
- Around line 1625-1671: Move the two reasoning normalization tests from the
package tests directory into a *.test.ts file alongside the adapter source
covered by createAdapter and chat. Preserve both test cases and their assertions
unchanged, updating only imports or references required by the new location.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cfb8dca-6cb0-40da-ad59-4a30ec2479d3
📒 Files selected for processing (8)
.changeset/openrouter-disable-reasoning.mdpackages/ai-openrouter/src/adapters/text.tspackages/ai-openrouter/src/text/text-provider-options.tspackages/ai-openrouter/tests/openrouter-adapter.test.tstesting/e2e/fixtures/reasoning/openrouter-disable.jsontesting/e2e/src/routeTree.gen.tstesting/e2e/src/routes/api.openrouter-reasoning-wire.tstesting/e2e/tests/openrouter-reasoning-wire.spec.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@testing/e2e/tests/openrouter-reasoning-wire.spec.ts`:
- Around line 27-35: Update the captured journal entry assertion in the scenario
check to first require that captured is defined and that its body is an object,
before validating the reasoning property. Keep the existing enabled and disabled
reasoning expectations unchanged after these presence assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dbbdcb27-8776-4677-b8dd-46401472f2ab
📒 Files selected for processing (3)
testing/e2e/fixtures/reasoning/openrouter-disable.jsontesting/e2e/src/routes/api.openrouter-reasoning-wire.tstesting/e2e/tests/openrouter-reasoning-wire.spec.ts
tombeckenham
force-pushed
the
fix/openrouter-empty-reasoning
branch
from
August 10, 2026 09:09
79b06b0 to
8b86c53
Compare
|
View your CI Pipeline Execution ↗ for commit 75224ca
☁️ Nx Cloud last updated this comment at |
Contributor
|
Thanks for the PR, @kinKingen! 🙌 @AlemTuzlak will take a look. Automated pre-review checks
Automated triage — a human review follows. |
github-actions
Bot
added
the
waiting-on: author
tombeckenham
force-pushed
the
fix/openrouter-empty-reasoning
branch
2 times, most recently
from
August 20, 2026 10:52
5caab67 to
387cf83
Compare
tombeckenham
force-pushed
the
fix/openrouter-empty-reasoning
branch
from
August 21, 2026 03:29
387cf83 to
75224ca
Compare
AlemTuzlak
reviewed
Aug 21, 2026
AlemTuzlak
left a comment
AlemTuzlak
left a comment
Contributor
There was a problem hiding this comment.
I treated this diff as invasive and checked whether each piece earns its keep.
The bug is real, and this mapping fixes it.
@openrouter/sdk@0.13.20 ChatRequestReasoning$outboundSchema still turns { enabled: false } into {}. { effort: 'none' } survives. That matches #1006.
I ran the two new unit tests both ways:
- without
normalizeReasoningOptions, they fail ({ enabled: false }is passed through;reasoning: {}is sent) - with the helper, they pass, including
ChatRequest$outboundSchema.parse(...)
Keep
normalizeReasoningOptions: mappingenabled: falsetoeffort: 'none'is what actually turns reasoning off. If we only dropenabledand omit the empty object, OpenRouter uses default reasoning.enabled?: falseonReasoningOptions: callers can pass the documented OpenRouter flag.- the two unit tests plus
ChatRequest$outboundSchema.parse: that is the serializer that caused the bug. - the changeset.
- the
as ExternalTextProviderOptionscast: I tried to drop it.tscfails because some per-model option unions omitreasoning.
Keep, but it is extra
- the dedicated E2E route, fixture, spec, and
routeTree.gen.tsupdate. The unit tests already parse the SDK outbound schema. The E2E hits HTTP afterHTTPClient. Repo policy wants E2E for behavior changes, so I would leave it.
Do not do
- CodeRabbit's request to move unit tests next to
src/.CONTRIBUTING.mdsayspackages/<pkg>/tests/.
Nits, not blockers
- The
enabledJSDoc restates the SDK bug. One line is enough: the adapter sends this aseffort: 'none'. docs/adapters/openrouter.mdhas no reasoning section. A shortmodelOptions.reasoningexample would help callers who copyenabled: falsefrom OpenRouter docs.
CodeRabbit threads are already resolved. Preview CI is a pkg.pr.new 404 on the fork, not this change.
AlemTuzlak
approved these changes
Aug 21, 2026
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-snippets
@tanstack/ai-codex
@tanstack/ai-cohere
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-daytona
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-isolate-quickjs-bun
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-perplexity
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vercel-gateway
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Fixes #1006.
reasoning: { enabled: false }requests by normalizing them to the SDK-supportedreasoning: { effort: 'none' }.reasoning: {}.@tanstack/ai-openrouter.Root cause
The adapter accepted OpenRouter's
reasoning.enabledoption and passed it to@openrouter/sdk@0.13.20. The generated SDK serializer does not include theenabledfield, so{ enabled: false }became an emptyreasoning: {}object on the wire. OpenRouter then applied its default reasoning behavior, effectively re-enabling what the caller explicitly disabled.The fix removes the unsupported field before serialization, maps explicit
falsetoeffort: 'none', and omits the object when no supported fields remain. The SDK version stays unchanged.Validation
pnpm test:prreasoning: { effort: 'none' }✅ Checklist
pnpm run test:pr.🚀 Release Impact
AI assistance disclosure
This contribution was developed with AI assistance. I reviewed the implementation, tests, and validation results and take responsibility for the submitted changes.
Summary by CodeRabbit
reasoning: { enabled: false }to the SDK-supported wire format (reasoning: { effort: 'none' }).reasoning: {}) are no longer included in outbound request payloads.